[WIP] Fix race condition in refresh token issuance by pavinduLakshan · Pull Request #478 · asgardeo/javascript

pavinduLakshan · 2026-04-27T15:08:14Z

Purpose

This pull request updates the AuthenticationHelper class to ensure that only one access token refresh operation can be in progress at a time. It introduces logic to track and reuse an in-flight token refresh promise, preventing duplicate refresh requests and potential race conditions.

Concurrency improvements for access token refresh:

Added a private property _refreshAccessTokenPromise to track an ongoing access token refresh operation, ensuring that concurrent calls to refreshAccessToken share the same promise and do not trigger multiple refreshes.
Updated the refreshAccessToken method to check for an existing refresh promise and return it if present, otherwise create and store a new one. The promise is cleared after completion or error, ensuring proper cleanup. [1] [2]

Related Issues

N/A

Related PRs

N/A

Checklist

Followed the CONTRIBUTING guidelines.
Manual test round performed and verified.
Documentation provided. (Add links if there are any)
Unit tests provided. (Add links if there are any)

Security checks

Followed secure coding standards in http://wso2.com/technical-reports/wso2-secure-engineering-guidelines?
Confirmed that this PR doesn't commit any keys, passwords, tokens, usernames, or other secrets?

Summary by CodeRabbit

Release Notes

Bug Fixes
- Fixed duplicate token refresh requests when multiple requests occur simultaneously.
- Enhanced error reporting during authentication token refresh failures.

coderabbitai · 2026-04-27T15:08:27Z

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4614bf6e-b5f1-4283-8c1e-b41a6614182a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

📝 Walkthrough

Walkthrough

The refreshAccessToken method now deduplicates concurrent refresh attempts by caching the in-progress Promise in a protected field, returning it for duplicate requests. Error handling posts REFRESH_ACCESS_TOKEN_ERROR and re-throws; a finally block clears the cache after completion.

Changes

Cohort / File(s)	Summary
Access Token Refresh De-duplication `packages/browser/src/__legacy__/helpers/authentication-helper.ts`	Added protected field `_refreshAccessTokenPromise` to cache in-progress refresh promises, preventing concurrent duplicate requests. Wrapped refresh logic in async IIFE for promise management. Modified error handling to post event before re-throwing. Added finally block to clear cache after completion.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A clever rabbit hops with glee,
De-duping tokens, wild and free!
One promise caches, no more race,
Error events mark every place,
Finally clears the hoppy trace! 🎯

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: fixing a race condition in refresh token issuance by de-duplicating concurrent refresh attempts.
Description check	✅ Passed	The PR description covers the purpose with clear details on the implementation, includes all required template sections with appropriate content and checklist items, though security and testing checklists remain unchecked.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix_refresh_token_race_condition

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/browser/src/__legacy__/helpers/authentication-helper.ts`:
- Around line 179-205: The httpRequestAll path still bypasses the deduplication
by calling this._authenticationClient.refreshAccessToken() directly; change
httpRequestAll to call the class method refreshAccessToken() (the one that uses
_refreshAccessTokenPromise) instead of invoking
_authenticationClient.refreshAccessToken() so all refresh requests funnel
through the existing de-dupe logic; update any calls in httpRequestAll that
reference _authenticationClient.refreshAccessToken to use
this.refreshAccessToken(), ensuring the error handling and window.postMessage
behavior remains intact.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a6ac56a8-7ea4-4efe-8d3a-d8ead6667e9e

📥 Commits

Reviewing files that changed from the base of the PR and between c041aca and 5aead49.

📒 Files selected for processing (1)

packages/browser/src/__legacy__/helpers/authentication-helper.ts

asgardeo-github-bot · 2026-04-28T05:43:07Z

⚠️ No Changeset found

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go.

If these changes should result in a version bump, you need to add a changeset.

Refer Release Documentation to learn how to add a changeset.

Fix race condition in refresh token issuance

5aead49

pavinduLakshan marked this pull request as draft April 27, 2026 15:08

coderabbitai Bot reviewed Apr 27, 2026

View reviewed changes

Comment thread packages/browser/src/__legacy__/helpers/authentication-helper.ts

pavinduLakshan and others added 2 commits April 28, 2026 10:48

Address review comments

1915b4c

Print debug logs in test workflow

ae24f3f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Fix race condition in refresh token issuance#478

[WIP] Fix race condition in refresh token issuance#478
pavinduLakshan wants to merge 3 commits intomainfrom
fix_refresh_token_race_condition

pavinduLakshan commented Apr 27, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 27, 2026 •

edited

Loading

Review skipped

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

asgardeo-github-bot commented Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

pavinduLakshan commented Apr 27, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Related Issues

Related PRs

Checklist

Security checks

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

asgardeo-github-bot commented Apr 28, 2026

⚠️ No Changeset found

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pavinduLakshan commented Apr 27, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 27, 2026 •

edited

Loading